Reapply "audio: module-adapter: use objpool for allocation containers"#10550
Reapply "audio: module-adapter: use objpool for allocation containers"#10550lyakh wants to merge 2 commits intothesofproject:mainfrom
Conversation
lyakh
requested review from
dabekjakub,
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
There was a problem hiding this comment.
Pull request overview
This PR re-applies the object-pool–based allocation container tracking for the module adapter memory API (previously reverted), and updates host/unit-test builds and platform stubs to support the objpool dependency again.
Changes:
- Replaces module resource container chunk/free-list bookkeeping with
objpoolusage inmodule_adaptergeneric resource management. - Introduces/standardizes
sof_sys_heap_get()as a real function (not a POSIX inline), with test/platform stubs. - Updates several cmocka audio test targets to link
src/lib/objpool.c.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
zephyr/lib/alloc.c |
Removes unused includes (cleanup). |
test/cmocka/src/common_mocks.c |
Adds a weak sof_sys_heap_get() stub for unit test linking. |
test/cmocka/src/audio/volume/CMakeLists.txt |
Links objpool.c into volume test support library. |
test/cmocka/src/audio/mux/CMakeLists.txt |
Links objpool.c into mux test support library. |
test/cmocka/src/audio/mixer/CMakeLists.txt |
Links objpool.c into mixer test target. |
test/cmocka/src/audio/eq_iir/CMakeLists.txt |
Links objpool.c into eq_iir test support library. |
test/cmocka/src/audio/eq_fir/CMakeLists.txt |
Links objpool.c into eq_fir test support library. |
src/platform/library/lib/alloc.c |
Adds a sof_sys_heap_get() stub for the platform/library build. |
src/include/sof/audio/module_adapter/module/generic.h |
Switches module_resources to use struct objpool_head. |
src/audio/module_adapter/module/generic.c |
Migrates container allocation/free and iteration logic to objpool. |
posix/include/rtos/alloc.h |
Changes sof_sys_heap_get() from inline stub to external declaration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| int ret = free_contents(cb_arg->mod, container); | ||
|
|
||
| if (ret < 0) | ||
| comp_err(cb_arg->mod->dev, "Cannot free allocation %p", cb_arg->ptr); | ||
|
|
||
| container_put(cb_arg->mod, container); | ||
|
|
||
| return true; |
There was a problem hiding this comment.
mod_res_free() logs when free_contents() fails, but still returns the container to the pool and then returns true, causing objpool_iterate() to report success. This drops the resource record even though the underlying resource may not have been freed. Consider capturing free_contents()'s return value in the callback arg so the caller can return an error, and only returning the container to the pool when free_contents() succeeds.
There was a problem hiding this comment.
freeing allocation contents should actually never fail, so it isn't quite clear what to do in that theoretical case. Maybe just panic.
This reverts commit 6d2d581. Using an object pool for module allocation containers should now work again. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
No need to check `mask != 0` before the loop - the loop already does it. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
|
@lrudyX random HDA DMA strikes again - do we need to rerun or can we ignore? |
2 participants
This reverts commit 6d2d581. Using an object pool for module allocation containers should now work again.